Skip to content

refactor(render): move WgpuWrapper into bevy_utils #19303

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
May 27, 2025
Merged

refactor(render): move WgpuWrapper into bevy_utils #19303

merged 2 commits into from
May 27, 2025

Conversation

atlv24
Copy link
Contributor

@atlv24 atlv24 commented May 20, 2025

Objective

  • A step towards splitting out bevy_camera from bevy_render

Solution

  • Move a shim type into bevy_utils to avoid a dependency cycle
  • Manually expand Deref/DerefMut to avoid having a bevy_derive dependency so early in the dep tree

Testing

  • It compiles

bushrat011899
bushrat011899 suggested changes May 20, 2025
View reviewed changes
Copy link
Contributor

@bushrat011899 bushrat011899 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor changes for no_std compatibility (in case someone activates the atomics feature on wasm32v1-none for example). Otherwise, I think this is a good idea.

@bushrat011899
Copy link
Contributor

bushrat011899 commented May 20, 2025 •
edited
Loading

Just need to update line 24 of bevy_utils/Cargo.toml:

- std = ["alloc", "bevy_platform/std", "dep:thread_local"]
+ std = ["alloc", "bevy_platform/std", "dep:thread_local", "dep:send_wrapper"]

@bushrat011899 bushrat011899 added C-Code-Quality A section of code that is hard to understand or change M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide A-Utils Utility functions and types X-Uncontroversial This work is generally agreed upon D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 20, 2025
@github-actions GitHub Actions
Copy link
Contributor

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Please review the instructions for writing migration guides, then expand or revise the content in the migration guides directory to reflect your changes.

@atlv24
Copy link
Contributor Author

atlv24 commented May 20, 2025

I don't believe this needs a migration guide, WgpuWrapper wasn't exposed before.

@bushrat011899 bushrat011899 removed the M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label May 20, 2025
@bushrat011899
Copy link
Contributor

My mistake! I saw that it was pub and assumed so.

@atlv24 atlv24 requested a review from bushrat011899 May 20, 2025 15:25
bushrat011899
bushrat011899 approved these changes May 20, 2025
View reviewed changes
Copy link
Contributor

@bushrat011899 bushrat011899 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect!

@atlv24 atlv24 added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 20, 2025
@alice-i-cecile
Copy link
Member

@atlv24 merge conflicts, sorry!

@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 27, 2025
Merged via the queue into bevyengine:main with commit 1732c22 May 27, 2025
36 checks passed
atlv24 added a commit to atlv24/bevy that referenced this pull request Jul 21, 2025
@atlv24
revert bevyengine#19303
4b8aeee
@atlv24 atlv24 mentioned this pull request Jul 21, 2025
Merged
atlv24 added a commit to atlv24/bevy that referenced this pull request Jul 21, 2025
@atlv24
revert bevyengine#19303
760e3b5
github-merge-queue bot pushed a commit that referenced this pull request Jul 21, 2025
@atlv24
revert #19303, moving WgpuWrapper out of bevy_utils (#20220)
1e07b21 
# Objective

- the changes done in #19303 did not end up being necessary for
bevy_camera, because i found a better split. it was still a good
refactor, but lets leave it in bevy_render

## Solution

- partially revert by moving the extracted file to bevy_render

## Testing

- it compiles
@atlv24 atlv24 deleted the wgpu-wrapper branch July 23, 2025 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
2 more reviewers

@bushrat011899 bushrat011899 bushrat011899 approved these changes

@tychedelia tychedelia tychedelia approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
A-Utils Utility functions and types C-Code-Quality A section of code that is hard to understand or change D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Uncontroversial This work is generally agreed upon
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@atlv24 @bushrat011899 @alice-i-cecile @tychedelia